Validate ML-DSA keygen response against received length#464
Validate ML-DSA keygen response against received length#464yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the ML-DSA client key-generation path by validating that the received response frame is long enough to contain the claimed exported key payload, preventing the client from deserializing trailing/stale bytes from its packet buffer. It brings ML-DSA in line with the existing ECC/RSA/ML-KEM keygen response-length validation pattern and adds a regression test to cover malformed server responses.
Changes:
- Added a defensive bounds check in
_MlDsaMakeKey()to ensureres->lenfits within the received frame length before consuming response fields/payload. - Added a new scripted-transport test that simulates truncated-but-“OK” keygen responses and asserts the client returns
WH_ERROR_ABORTED. - Registered the new test in the Misc test group.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/wh_client_crypto.c |
Adds response-length validation for ML-DSA keygen before using res->keyId/res->len and before DER deserialization. |
test-refactor/misc/wh_test_client_malformed_resp.c |
New negative test using a scripted transport to simulate malformed/truncated ML-DSA keygen responses. |
test-refactor/wh_test_list.c |
Registers the new malformed-response test in the Misc test list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #464
Scan targets checked: wolfhsm-crypto-bugs, wolfhsm-src
No new issues found in the changed files. ✅
c7ab583 to
7903b1a
Compare
|
Issue 5462 was fixed by Commit 35bfcf1 in parallel. |
Summary
Two ML-DSA client response handlers consumed server-reported lengths without
checking them against the frame that actually arrived.
_MlDsaMakeKeyrecorded the response length fromwh_Client_RecvResponsebutnever used it. It read
res->keyIdandres->lenand, on the ephemeral exportpath, deserialized
res->lenbytes from(res + 1)without confirming theframe carried that payload. A short but otherwise well-formed OK response
therefore made the client consume whatever trailed the response body in its own
packet buffer.
wh_Client_MlDsaSignDmahad the same gap: it storedres->sigLeninto thecaller's
*out_lenwithout checking that the frame was long enough to containthe field, or that the reported length fit the caller's buffer.
ECC, RSA and ML-KEM keygen already validate this; ML-DSA was the last keygen
path without it.
_MlDsaMakeKeybacks bothwh_Client_MlDsaMakeCacheKeyandwh_Client_MlDsaMakeExportKey, and the export wrapper is reached from the PQCsignature crypto callback.
Fixes F_5462.
Changes
src/wh_client_crypto.cKeygen — bounds check matching the existing ECC/RSA/ML-KEM condition verbatim:
Placed up front, before any response field is consumed, so the
res->keyIdreadon the cache path is covered too. Cache responses carry no payload (
len == 0,frame ==
hdr_sz) and still pass.DMA sign — the response carries no inline payload, so the guard checks the frame
length and the reported signature length against the caller's capacity. That
capacity is stashed in
sig_capbefore the request is sent, because theresponse overwrites the request struct it was read from.
test-refactor/misc/wh_test_client_malformed_resp.c(new) — negative tests.These APIs send and receive in one blocking call, so they cannot be driven by a
raw comm server on a single thread. The test installs a scripted
whTransportClientCbthat echoes the request comm header and returns a responsewhose reported length is set independently of the frame length it delivers.
buffer), a headers-only frame claiming 256 DER bytes, and a frame shorter than
the crypto headers.
WOLFHSM_CFG_DMAonly): a complete reply that fits, a frame one byteshort of the response, and a complete frame reporting a signature larger than
the caller's buffer.
All malformed cases must return
WH_ERROR_ABORTED.test-refactor/wh_test_list.c— registered in the Misc group.Verification
Both guards were negative-controlled: with the check neutered the corresponding
subtest fails, and passes once restored.
make check38 passed, 25 skipped, 0 failed of 63make check DMA=142 passed, 21 skipped, 0 failed of 63The DMA sign subtest is compiled out without
DMA=1, so theDMA=1run is theone that exercises it. With the ML-DSA guard forced off, the entry point falls
through to the weak
WH_TEST_DECLstub and reports SKIPPED rather than an emptypass.